Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xcm SetAssetClaimer 1 #5585

Open
wants to merge 93 commits into
base: xcm-pay-fees
Choose a base branch
from
Open

Xcm SetAssetClaimer 1 #5585

wants to merge 93 commits into from

Conversation

x3c41a
Copy link

@x3c41a x3c41a commented Sep 4, 2024

Added SetAssetClaimer instruction

x3c41a and others added 30 commits September 4, 2024 13:14
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
@x3c41a
Copy link
Author

x3c41a commented Sep 5, 2024

bot bench polkadot-pallet --subcommand=xcm --runtime=westend --pallet=pallet_xcm_benchmarks::generic

@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@x3c41a https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7265736 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::generic. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 27-1aaf8bc7-2710-49b2-9960-7238f9315e94 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@x3c41a Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::generic has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7263985 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7263985/artifacts/download.

@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@x3c41a Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::generic has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7264123 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7264123/artifacts/download.

@x3c41a
Copy link
Author

x3c41a commented Sep 5, 2024

bot bench polkadot-pallet --subcommand=xcm --runtime=westend --pallet=pallet_xcm_benchmarks::generic

@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@x3c41a https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7266819 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::generic. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 29-9a396d75-6026-42b1-ae43-33a2f43e05a9 to cancel this command or bot cancel to cancel all commands in this pull request.

…stend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::generic
@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@x3c41a Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::generic has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7265736 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7265736/artifacts/download.

@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@x3c41a Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::generic has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7266819 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7266819/artifacts/download.

@kianenigma kianenigma changed the title Xcm sac 1 Xcm SetAssetClaimer 1 Sep 17, 2024
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7385049

Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks good, the unit tests look good. My ideal test for this would be:

  • User in penpal a wants to reserve asset transfer some WND to a user on penpal b (through AssetHub)
  • Sets amount for fees as too low and the assets are trapped on AssetHub
  • Thankfully he set the claimer
  • He sends a new message (using polkadotXcm::send because of the ClearOrigin problem with transfers) with the ClaimAsset instruction.
  • With the claimed assets he can finish the send to penpal b

@@ -98,6 +106,8 @@ mod imports {
pub type ParaToParaThroughRelayTest = Test<PenpalA, PenpalB, Westend>;
pub type ParaToParaThroughAHTest = Test<PenpalA, PenpalB, AssetHubWestend>;
pub type RelayToParaThroughAHTest = Test<Westend, PenpalA, AssetHubWestend>;
pub type BridgeToAssetHubTest = Test<BridgeHubWestend, AssetHubWestend>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub type BridgeToAssetHubTest = Test<BridgeHubWestend, AssetHubWestend>;
pub type BridgeHubToAssetHubTest = Test<BridgeHubWestend, AssetHubWestend>;

let xcm_here = Xcm::<RuntimeCall>::builder_unsafe()
.withdraw_asset((Parent, 200_000_000_000u128))
.pay_fees((Parent, 40_000_000_000u128))
.export_message(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This instruction is only used to send a message across a bridge. There's no need to use it here

@@ -343,4 +343,12 @@ impl<T: frame_system::Config> pallet_xcm::WeightInfo for WeightInfo<T> {
.saturating_add(T::DbWeight::get().reads(1))
.saturating_add(T::DbWeight::get().writes(1))
}

fn set_asset_claimer() -> Weight {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here in pallet_xcm, since it's not a call you can make on the pallet. It's correct to have it in pallet-xcm-benchmarks because we use that for xcm instructions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

Comment on lines +185 to +187
fn set_asset_claimer() -> Weight {
Weight::from_parts(2_176_000, 0)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed. There's no call in pallet_xcm like this. This trait has a function for each pallet call but returns a weight.

let result = vm.bench_process(xcm);
assert!(result.is_err());
assert_eq!(vm.asset_claimer(), Some(bob));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New line at end of file 😁

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this is marked as changed. Maybe a space?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants